Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🔧 [open-zaak/open-zaak#1629] Refactor settings module #102

Merged
merged 9 commits into from
May 17, 2024

Conversation

stevenbal
Copy link
Contributor

@stevenbal stevenbal commented May 1, 2024

partially fixes open-zaak/open-zaak#1629
Related PR: maykinmedia/open-api-framework#19 (once that is merged, I'll do a release and remove the git dependency from Objecttypes)

I moved the settings from base.py to a generic base.py in Open API Framework (see maykinmedia/open-api-framework#19). I'm not sure if it will be possible to do the same for the environment specific settings (production, dev, docker etc.), but that might be worth looking into at some point.

I also had to make several fixes with regards to the docker-compose setup and OIDC.

Some things that I noticed:

  • Objecttyped used VERSION_TAG, I changed this to RELEASE
  • TIME_ZONE and LANGUAGE_CODE differ from Open Zaak

@stevenbal stevenbal marked this pull request as draft May 1, 2024 10:18
@stevenbal stevenbal force-pushed the feature/refactor-settings branch 2 times, most recently from 0e4f269 to f3d54a4 Compare May 7, 2024 10:39
stevenbal added 6 commits May 7, 2024 14:31
use base settings from open-api-framework and add staging/production settings
* fix interpolation issue in docker-compose
* upgrade postgres to 12
* add redis container
* add DISABLE_2FA envvar
* add CACHE_DEFAULT and CACHE_AXES
_fonts.scss relied on font assets that were in Django 2.2.x, but this causes errors when using ManifestStaticFilesStorage
as it was previously broken using 0.14.1
@stevenbal stevenbal force-pushed the feature/refactor-settings branch from fb186af to 95b0f4c Compare May 7, 2024 12:31
@stevenbal stevenbal marked this pull request as ready for review May 7, 2024 12:40
@stevenbal stevenbal requested review from annashamray and Coperh May 7, 2024 12:40
Copy link
Contributor

@Coperh Coperh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

All of the config variables are in the open-api-framework as far as I can tell.

src/objecttypes/conf/base.py Show resolved Hide resolved
Copy link
Collaborator

@annashamray annashamray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cleaned conf/base.py is 🤌

docker-compose.yml Show resolved Hide resolved
FIXTURE_DIRS = (os.path.join(DJANGO_PROJECT_DIR, "fixtures"),)

LOGGING_DIR = os.path.join(BASE_DIR, "log")
LANGUAGE_CODE = "en-us" # FIXME should this be "nl-nl"?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question! Maybe @joeribekker could help us?

src/objecttypes/conf/base.py Outdated Show resolved Hide resolved
* remove unused setting
* add warning in changelog to add Redis
@stevenbal stevenbal requested a review from annashamray May 14, 2024 09:21
Copy link
Collaborator

@annashamray annashamray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work!

@stevenbal stevenbal merged commit 07c57b2 into master May 17, 2024
11 checks passed
@stevenbal stevenbal deleted the feature/refactor-settings branch May 17, 2024 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

As PO I want to streamline all environment variables.
3 participants